Instead of special-casing 1.8.0-alpha.0, could we say k8sVersion < 1.8?

Problem is that v1.8.0-alpha.1 will be perfectly valid...

I'm not sure that I fully grok what happened here, but I think some new unit tests would be a great way to demonstrate what the expected behavior is (as well as prove that this works how we think it does).

Is the problem that this feature is going to be removed in 1.8, or is there actually a problem with how we do semver and we think that 1.8.0-alpha.0 came before 1.7.0? My interpretation of this PR is the latter, which is actually pretty scary if true (and something we should excalate / fix project-wide, I think).

Okay, after looking at the repository version tags, I understand now. Ideally, semver means that 1.8.0-alpha.0 came strictly after anything in 1.7, and our engine handles that correctly, but we have actually applied the version tag to the same commit as 1.7.0-beta.0, so the semver comparison doesn't actually make sense, and 1.7.1-beta.1 actually came after 1.8.0-alpha.0 in the commit history. Yuck.

Since the release cycles for wrapping up 1.7 and pushing forward with 1.8 are overlapping, I wonder if the better solution (instead of special-casing 1.8.0-alpha.0) is to have a minimum 1.8 version to check as well. Someone could still tag 1.8.0-alpha.1 to be a commit that logically occurred before 1.7.0-beta.1, but it should at least be a commit later than 1.8.0-alpha.0. I hope that we at least have strong semver guarantees within each 1.X lineage.

So, I'd recommend changing this from an exact check of 1.8.0-alpha.0 to instead check for >= 1.8.0-alpha.1. That way, we're logically asking "is this greater than the first release that included the feature in the 1.7 or 1.8 series?" instead of saying "1.8.0-alpha.0 is a weird botched special case we have to handle specifically."

If the logic gets complicated, I would also pull it out into a well-named helper like IsNodeAuthorizerSupported that we can much more thoroughly test for all of the version edge cases.

I'll make IsNodeAuthorizerSupported instead tomorrow...
Are you fine with
if version.AtLeast("v1.7.0-beta.1") && version != "v1.8.0-alpha.0" or do you strictly want
if (version.AtLeast("v1.7.0-beta.1") && version.LessThan("v1.8.0-alpha.0")) || version.AtLeast("v1.7.0-beta.1")?

I think they'll produce the same logic anyway...

1.7.1-beta.1 actually came after 1.8.0-alpha.0 in the commit history. Yuck.

wait, what? Why are we cutting 1.8 anything?

@timothysc Not sure how/why it got cut, but it exists: https://github.com/kubernetes/kubernetes/releases/tag/v1.8.0-alpha.0

@luxas What I was originally suggesting was:

if version.AtLeast("v1.7.0-beta.1") || version.AtLeast("v1.8.0-alpha.1")

but I realized after writing it that it wouldn't skip v1.8.0-alpha.0 at all, unless we do something more elaborate, like:

if (version.Prefix("v1.7") && version.AtLeast("v1.7.0-beta.1")) || version.Prefix("v1.8") && version.AtLeast("v1.8.0-alpha.1")

I don't really mind about how we write the if as long as the intent is clear (by having a well-named helper instead of just a complex inline if), there are adequate tests to avoid an oops, and the reasoning behind the version juggling is explained well in comments.